Enhancement proposal: "Add Providers Support" #2272
Conversation
|
Can one of the admins verify this patch? |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci
bot
added
the
size/L
There was a problem hiding this comment.
| Implement a function which determines which provider should be used. | |
| Implement a function which determines which providers can be used. |
There was a problem hiding this comment.
Regarding the can/should I think that can be the provider's decision on how to implement, but it is an interesting question to ask.
If there is a use case in which the user will provision the Nodes on a certain provider but want to disable provider integration? I'll add that to the open questions
There was a problem hiding this comment.
Sure. We see that today. Maybe they don't have platform credentials, don't want to provide us with the credentials, or they are doing some custom integrations.
There was a problem hiding this comment.
The generation of the infraID is random but it can be known as in AWS UPI: https://github.com/openshift/installer/blob/master/docs/user/aws/install_upi.md#extract-infrastructure-name-from-ignition-metadata
Extracting the infra id would be preferred if it satisfies the needs. If it does not, let's discuss why.
There was a problem hiding this comment.
In the assisted installer the machines are provisioned before the ignition-configs or manifests are generated.
If we will go with this approach we will need to make manual modifications to the User Provisioned Nodes
There was a problem hiding this comment.
How does it work with UPI?
There was a problem hiding this comment.
Or we can chagne to the ignition config rendered by the installer 😉
There was a problem hiding this comment.
@eranco74 it also affects the manifests a lot.
@avishayt On current RHV UPI we don't enable provider integration, we have a separate effort to document this as well, but essentially since the manifests and ignition can be created before there are any Nodes there is no problem with extracting the infraID and then creating the VMs with that infraID in the name
Gal-Zaidman
force-pushed
the
add-external-providers-enhancement
branch
from
f4007e3 to
2e8f82d
Compare
mhrivnak
left a comment
mhrivnak
left a comment
There was a problem hiding this comment.
Thanks for getting this started.
Am I understanding your intent correctly that this does not propose any changes to how a discovery image gets booted on a host, but only affects how a cluster definition represents platform integration and how that integration is implemented? I think it's important to clarify that scope prominently in the proposal.
There was a problem hiding this comment.
It would be an improvement to rephrase this as an outcome rather than a solution. The goal is to enable additional platforms in some way, so that a user can create a cluster with platform integration. What you're describing here is solution details that would be better in the Proposal section.
There was a problem hiding this comment.
OK I agree, moved to the proposal part and replaced with:
- Define a clear way to add a specific platform provider to the assisted installer.
- Add the oVirt platform provider to the assisted installer.
- Migrate all existing supported providers to implement the same unified interface.
Is that better?
There was a problem hiding this comment.
Is this (multiple platforms in a single cluster) something the openshift installer support?
There was a problem hiding this comment.
Or we can chagne to the ignition config rendered by the installer 😉
flaper87
left a comment
flaper87
left a comment
There was a problem hiding this comment.
Sending these comments for now. Will review again later. :)
|
Updated some comments, |
|
@Gal-Zaidman: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@YuviGold @mhrivnak I added all the changes in a separate commit so it will be easier to review |
Gal-Zaidman
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Gal-Zaidman
force-pushed
the
add-external-providers-enhancement
branch
from
31b6701 to
3227b67
Compare
Gal-Zaidman
force-pushed
the
add-external-providers-enhancement
branch
from
3227b67 to
cdf69a9
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Gal-Zaidman, YuviGold The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
13 participants
Currently, the assisted installer supports Bare-Metal installations only.
Openshift can be installed on various providers such as:
This enhancement will define a clear interface to extend the assisted installer and allow
provider installations.